refactor(brain): simplify pass on tracked ops scripts#39
Conversation
- Extract shared Ollama /api/generate wrapper + SDK-path wiring into brain/scripts/_common.py (only consolidation with >=2 call sites). - ab_test_constitutional + mirofish_sim: delegate to ollama_generate, drop duplicated urllib plumbing. - brain_benchmark: use ensure_sdk_on_path(); delete dead first-pass defaultdict loop in _simulate_graduation (was a no-op overwritten by the correct second loop). - mirofish_sim: remove dead Forum.add_disagree() no-op and the unused ``field`` import. CLI signatures preserved. All 2060 tests still pass.
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (4)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Cloudflare Pages failure: unrelated to this PR, safe to mergeRoot cause: The failing check is for the Evidence this is pre-existing on
Other checks on this PR: CodeRabbit pass, test (3.11/3.12/3.13) pass. Recommendation: Merge this refactor PR. The |
Conflicts in cloud/app/db.py, routes/brains.py, routes/users.py. Resolution strategy (preserve main's behavior additions, keep simplify intent where non-conflicting): - db.py: accepted main's explicit in_= parameter (required by activity.py, rule_patches.py, brains.py callers merged via #44). Kept simplify's _raise_db_error for uniform error handling. - routes/brains.py: kept main's batched lessons+corrections IN-query for list_brains (perf) + main's new POST /brains endpoint. Preserved simplify's _brain_detail helper, Depends(get_brain_for_request) pattern, and _is_demo consolidation. - routes/users.py: accepted main's version wholesale (adds email, _derive_plan, _primary_workspace_id scoped notification updates). Simplify's _hydrate_workspaces helper dropped — main's inline form carries the required new behavior and this PR is a refactor (no behavior changes). No new behavior introduced by this merge commit.
Summary
/api/generatewrapper + SDK sys.path wiring across three scripts into newbrain/scripts/_common.pybrain_benchmark._simulate_graduation, no-opForum.add_disagreeand unusedfieldimport inmirofish_simscore_brain,Agent,Post,Forum,_generate,_ollama_generatedirectly)Touched
brain/scripts/_common.py(new, 2 shared helpers)brain/scripts/ab_test_constitutional.pybrain/scripts/brain_benchmark.pybrain/scripts/mirofish_sim.pycross_validate.py,migrate_tree_paths.py,optimization_runner.py— already tight, no shared patterns worth extractingTest plan
--helpon every script still worksab_test_constitutional.py --dry-runprintsimports = OKpytest tests/— 2060 passed, 23 skippedGenerated with Gradata